Validate the optimistic lock in INSERT ON CONFLICT DO UPDATE#6306
Validate the optimistic lock in INSERT ON CONFLICT DO UPDATE#6306eyupcanakman wants to merge 1 commit into
Conversation
For the runtime validation to return the rows updated it would have a where clause for version lock
Yes, I think so because it's related to the runtime validation and checking the row affected Current issues I can see: Duplication of So, there needs to be a way to keep the validation in a single place. The docs can be updated in another PR. |
ad7ab33 to
eeac5ba
Compare
|
Both setter forms still work, but the lock now has to be checked in a WHERE clause either way, self increment included. The runtime throw wants your single validation first I think, since |
Thanks - do it as a stacked PR. CREATE TABLE test (
id INTEGER AS VALUE NOT NULL,
other INTEGER NOT NULL,
version INTEGER AS LOCK NOT NULL DEFAULT 0,
text TEXT NOT NULL,
UNIQUE (id),
UNIQUE (other)
);
upsertText:
INSERT INTO test (id, other, text)
VALUES (:id, :other, :text)
ON CONFLICT (id) DO UPDATE
SET text = :text, version = version + 1
WHERE version = :version
ON CONFLICT (other) DO UPDATE
SET text = :text, version = version + 1
WHERE version = :version;e.g Result |
The optimistic lock check only ran for UPDATE statements, so an INSERT with ON CONFLICT DO UPDATE skipped it. A table with an AS LOCK column would compile an upsert whose DO UPDATE SET never touches the lock. The same check now runs on the DO UPDATE SET clause in the sqlite-3-24 and sqlite-3-35 dialects. DO UPDATE also has to carry the WHERE clause a multirow UPDATE can leave out. Self incrementing the lock bumps every row an UPDATE matches, but DO UPDATE only ever touches the row that conflicted, so without WHERE version = :version the stored lock is never read. Upserts need one more guard, because excluded is a real query source there. SET version = excluded.version + 1 and WHERE excluded.version = :version both read back the caller's own incoming value, so neither counts as the stored lock. The table name and the table alias do. Refs sqldelight#6232
eeac5ba to
277ee5b
Compare
|
Added the sqlite 3_35 test with two conflict clauses, both carrying the version check, and it validates clean. Left the runtime throw for the stacked follow-up. |
Thanks, the two issues for the follow up are:
|
2 participants
Refs #6232
OptimisticLockValidatoronly pattern-matchesSqlUpdateStmtandSqlUpdateStmtLimited, so anINSERT ... ON CONFLICT DO UPDATEnever reached the lock check.The
DO UPDATE SETclause below drops theversioncolumn declaredAS LOCK, and it compiles:The checks the UPDATE path applies now run against
DO UPDATE SETtoo, in the sqlite-3-24 and sqlite-3-35 dialects.Postgres and MySQL upserts have no
WHEREin theirDO UPDATEgrammar, so the same check is a separate question there and stays out of this PR, which is why this isRefsand notCloses.An UPDATE cannot express
excluded, which holds the row being inserted rather than the stored one.SET version = excluded.version + 1would pass as a self increment if you only compare column names, so both checks now read the lock from the target table or its alias.Two things I would like your read on, @griffio.
I kept the self increment from #6240, so
SET version = version + 1skips theWHEREcheck here too.An upsert's
DO UPDATEonly touches the row that conflicted, so the multirow reasoning does not carry over.Should an upsert require the bind form?
QueryGeneratorwires the lock only forNamedMutator.Update, so a stale locked upsert affects zero rows without throwingOptimisticLockException.I can pick that up here or in a follow-up.
Docs are untouched. The optimistic locking section is only included on the H2, MySQL and Postgres pages, never the SQLite ones.
CHANGELOG.md's "Unreleased" section has been updated, if applicable.